Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Block Extension QSL #100

Draft
wants to merge 15 commits into
base: 1.18
Choose a base branch
from
Draft

Block Extension QSL #100

wants to merge 15 commits into from

Conversation

Azagwen
Copy link

@Azagwen Azagwen commented Apr 19, 2022

This PR contains the base for a QuiltBlock Utility, which currently only has Property proxying (i.e creating blocks that "extend" multiple blocks, thus inheriting their blockstate properties)

To create a block proxy, you simply call QuiltBlock.createProxy() in its constructor, as TestBlock for the example mod shows.
QuiltBlock.createProxy() takes the block's settings, and an array of already registered blocks, the block settings are only used to be able to call the method itself, as this needs to happen at the super() call

public class TestBlock extends Block {

    public TestBlock(Settings settings, Block... proxies) {
        super(QuiltBlock.createProxy(settings, proxies));
    }

}

@TheGlitch76 TheGlitch76 marked this pull request as draft April 19, 2022 01:12
… a property from a block with proxies, without crashing the game if the peroperty doesn't exist.
Copy link
Contributor

@LambdAurora LambdAurora left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some notes:

  • we use <Class>Mixin and not Mixin<Class>
  • I feel like copyProperty should do checks to avoid copying something that cannot be copied
  • getProxyDefaultState probably should be replaced with a more generic method called mergeStates(BlockState source, BlockState target).

Also don't forget to run the applyLicenses task!


@Override
public ActionResult onUse(BlockState state, World world, BlockPos pos, PlayerEntity player, Hand hand, BlockHitResult hit) {
var prop = QuiltBlock.getProperty(state, StairsBlock.HALF);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This usage of getProperty doesn't sound useful?

If the stuff that was merged were merged properly, it should already include the property.

@Azagwen
Copy link
Author

Azagwen commented Apr 23, 2022

Some notes:

  • we use <Class>Mixin and not Mixin<Class>
  • I feel like copyProperty should do checks to avoid copying something that cannot be copied
  • getProxyDefaultState probably should be replaced with a more generic method called mergeStates(BlockState source, BlockState target).

Also don't forget to run the applyLicenses task!

How would you generify getProxyDefaultState?

I struggle to see any ways to 100% generify this method in particular, as it's currently not possible to target a single Proxy and it needs to get all of them in a loop to gather teh default states.

Unless I create a new method to mergeStates and use it in a refactor of getProxyDefaultState which does loop through proxies?

@Azagwen Azagwen changed the title Base QuiltBlock Impl for Property proxying Block Extension QSL Apr 24, 2022
@LambdAurora LambdAurora added library: block Related to the block library. s: wip This pull request is being worked on. labels May 20, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
library: block Related to the block library. s: wip This pull request is being worked on.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants